fix: pre-warm node-gyp header cache in all build images#68
Conversation
dsanders11
left a comment
There was a problem hiding this comment.
Opus 4.6 review with a bit of direction from me.
You're not wrong. PR #68 is significantly overengineered.
PR #68 (build-images) writes a 74-line bash script that manually reimplements what node-gyp install already does:
- Manually downloads header tarballs from
nodejs.org - Manually verifies SHA256 checksums
- Manually extracts and mirrors the on-disk cache layout
- Writes a fake
installVersionmarker set to99to trick node-gyp into treating the cache as valid across future versions
PR #38287 (electron/electron) solved the same race condition problem with ~15 lines by just running node-gyp install with the right env vars (npm_config_devdir, npm_config_disturl, npm_config_target).
The build-images Dockerfile could replace the entire script with something like:
ENV npm_config_devdir=/opt/node-gyp-cache
RUN node -e "const v = process.version.slice(1); \
require('child_process').execSync( \
'node-gyp install --ensure --target=' + v, \
{stdio: 'inherit'})" \
&& chmod -R a+rwX /opt/node-gyp-cacheThe hardcoded INSTALL_VERSION_MARKER=99 is particularly fragile — it's betting on node-gyp's internal cache-validation logic never changing incompatibly, while the comment even acknowledges that risk. Letting node-gyp install write its own marker is inherently correct and forward-compatible.
The only stated justification for avoiding node-gyp is "rather than installing node-gyp itself just to run it once during image build" — but node-gyp ships bundled with npm, which is already installed in the image. There's nothing extra to install.
Parallel yarn postinstall builds of the spec native-addon fixtures race each other downloading node headers into $HOME/.cache/node-gyp/<version>/, which has no locking. The race can leave a half-written node_api_types.h and fail downstream fixture compiles with "'napi_status' does not name a type". Populate the cache on the CI host with a lockfile-pinned node-gyp before docker build, then COPY it into every image at /opt/node-gyp-headers and point node-gyp there via npm_config_devdir. The headers tarball is arch-independent on Linux (node.lib is Windows-only), so generating once on the amd64 runner is safe for the arm images too.
ed1a643 to
b88ac9b
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Co-authored-by: David Sanders <dsanders11@ucsbalum.com>
- Add .github/actions/build-image-sha as the single source of truth for the ghcr.io/electron/build (and arch-tagged electron/test) image SHA, with an optional override input for workflow_dispatch. - Refactor build.yml, apply-patches.yml, build-git-cache.yml, clean-src-cache.yml, clean-orphaned-cache-uploads.yml, and the three publish workflows to resolve the SHA via a small ubuntu-slim setup job instead of hardcoding it in each file. - Bump the image to daad061f (electron/build-images#68, which pre-warms the node-gyp header cache in the Linux images). - Run the build.yml setup job on ubuntu-slim instead of ubuntu-latest. - In install-dependencies (and the inline yarn installs in pipeline-electron-lint and generate-types), link deps with --mode=skip-build first, run `node-gyp install` with up to 3 retries (5s backoff) to populate the header cache, then run the build phase. This avoids the parallel-download race that intermittently fails the first native-addon configure with an empty common.gypi on cold macOS/Windows runners.
* ci: centralize build-image SHA and pre-seed node-gyp headers - Add .github/actions/build-image-sha as the single source of truth for the ghcr.io/electron/build (and arch-tagged electron/test) image SHA, with an optional override input for workflow_dispatch. - Refactor build.yml, apply-patches.yml, build-git-cache.yml, clean-src-cache.yml, clean-orphaned-cache-uploads.yml, and the three publish workflows to resolve the SHA via a small ubuntu-slim setup job instead of hardcoding it in each file. - Bump the image to daad061f (electron/build-images#68, which pre-warms the node-gyp header cache in the Linux images). - Run the build.yml setup job on ubuntu-slim instead of ubuntu-latest. - In install-dependencies (and the inline yarn installs in pipeline-electron-lint and generate-types), link deps with --mode=skip-build first, run `node-gyp install` with up to 3 retries (5s backoff) to populate the header cache, then run the build phase. This avoids the parallel-download race that intermittently fails the first native-addon configure with an empty common.gypi on cold macOS/Windows runners. * ci: skip node-gyp header pre-seed on Linux * ci: invoke node-gyp via its JS entrypoint for Windows compat
* ci: centralize build-image SHA and pre-seed node-gyp headers - Add .github/actions/build-image-sha as the single source of truth for the ghcr.io/electron/build (and arch-tagged electron/test) image SHA, with an optional override input for workflow_dispatch. - Refactor build.yml, apply-patches.yml, build-git-cache.yml, clean-src-cache.yml, clean-orphaned-cache-uploads.yml, and the three publish workflows to resolve the SHA via a small ubuntu-slim setup job instead of hardcoding it in each file. - Bump the image to daad061f (electron/build-images#68, which pre-warms the node-gyp header cache in the Linux images). - Run the build.yml setup job on ubuntu-slim instead of ubuntu-latest. - In install-dependencies (and the inline yarn installs in pipeline-electron-lint and generate-types), link deps with --mode=skip-build first, run `node-gyp install` with up to 3 retries (5s backoff) to populate the header cache, then run the build phase. This avoids the parallel-download race that intermittently fails the first native-addon configure with an empty common.gypi on cold macOS/Windows runners. * ci: skip node-gyp header pre-seed on Linux * ci: invoke node-gyp via its JS entrypoint for Windows compat (cherry picked from commit f7ba340)
…#51277) ci: centralize build-image SHA and pre-seed node-gyp headers (#51148) * ci: centralize build-image SHA and pre-seed node-gyp headers - Add .github/actions/build-image-sha as the single source of truth for the ghcr.io/electron/build (and arch-tagged electron/test) image SHA, with an optional override input for workflow_dispatch. - Refactor build.yml, apply-patches.yml, build-git-cache.yml, clean-src-cache.yml, clean-orphaned-cache-uploads.yml, and the three publish workflows to resolve the SHA via a small ubuntu-slim setup job instead of hardcoding it in each file. - Bump the image to daad061f (electron/build-images#68, which pre-warms the node-gyp header cache in the Linux images). - Run the build.yml setup job on ubuntu-slim instead of ubuntu-latest. - In install-dependencies (and the inline yarn installs in pipeline-electron-lint and generate-types), link deps with --mode=skip-build first, run `node-gyp install` with up to 3 retries (5s backoff) to populate the header cache, then run the build phase. This avoids the parallel-download race that intermittently fails the first native-addon configure with an empty common.gypi on cold macOS/Windows runners. * ci: skip node-gyp header pre-seed on Linux * ci: invoke node-gyp via its JS entrypoint for Windows compat (cherry picked from commit f7ba340)
…#51276) ci: centralize build-image SHA and pre-seed node-gyp headers (#51148) * ci: centralize build-image SHA and pre-seed node-gyp headers - Add .github/actions/build-image-sha as the single source of truth for the ghcr.io/electron/build (and arch-tagged electron/test) image SHA, with an optional override input for workflow_dispatch. - Refactor build.yml, apply-patches.yml, build-git-cache.yml, clean-src-cache.yml, clean-orphaned-cache-uploads.yml, and the three publish workflows to resolve the SHA via a small ubuntu-slim setup job instead of hardcoding it in each file. - Bump the image to daad061f (electron/build-images#68, which pre-warms the node-gyp header cache in the Linux images). - Run the build.yml setup job on ubuntu-slim instead of ubuntu-latest. - In install-dependencies (and the inline yarn installs in pipeline-electron-lint and generate-types), link deps with --mode=skip-build first, run `node-gyp install` with up to 3 retries (5s backoff) to populate the header cache, then run the build phase. This avoids the parallel-download race that intermittently fails the first native-addon configure with an empty common.gypi on cold macOS/Windows runners. * ci: skip node-gyp header pre-seed on Linux * ci: invoke node-gyp via its JS entrypoint for Windows compat (cherry picked from commit f7ba340)
…#51275) ci: centralize build-image SHA and pre-seed node-gyp headers (#51148) * ci: centralize build-image SHA and pre-seed node-gyp headers - Add .github/actions/build-image-sha as the single source of truth for the ghcr.io/electron/build (and arch-tagged electron/test) image SHA, with an optional override input for workflow_dispatch. - Refactor build.yml, apply-patches.yml, build-git-cache.yml, clean-src-cache.yml, clean-orphaned-cache-uploads.yml, and the three publish workflows to resolve the SHA via a small ubuntu-slim setup job instead of hardcoding it in each file. - Bump the image to daad061f (electron/build-images#68, which pre-warms the node-gyp header cache in the Linux images). - Run the build.yml setup job on ubuntu-slim instead of ubuntu-latest. - In install-dependencies (and the inline yarn installs in pipeline-electron-lint and generate-types), link deps with --mode=skip-build first, run `node-gyp install` with up to 3 retries (5s backoff) to populate the header cache, then run the build phase. This avoids the parallel-download race that intermittently fails the first native-addon configure with an empty common.gypi on cold macOS/Windows runners. * ci: skip node-gyp header pre-seed on Linux * ci: invoke node-gyp via its JS entrypoint for Windows compat (cherry picked from commit f7ba340)
…#51273) ci: centralize build-image SHA and pre-seed node-gyp headers (#51148) * ci: centralize build-image SHA and pre-seed node-gyp headers - Add .github/actions/build-image-sha as the single source of truth for the ghcr.io/electron/build (and arch-tagged electron/test) image SHA, with an optional override input for workflow_dispatch. - Refactor build.yml, apply-patches.yml, build-git-cache.yml, clean-src-cache.yml, clean-orphaned-cache-uploads.yml, and the three publish workflows to resolve the SHA via a small ubuntu-slim setup job instead of hardcoding it in each file. - Bump the image to daad061f (electron/build-images#68, which pre-warms the node-gyp header cache in the Linux images). - Run the build.yml setup job on ubuntu-slim instead of ubuntu-latest. - In install-dependencies (and the inline yarn installs in pipeline-electron-lint and generate-types), link deps with --mode=skip-build first, run `node-gyp install` with up to 3 retries (5s backoff) to populate the header cache, then run the build phase. This avoids the parallel-download race that intermittently fails the first native-addon configure with an empty common.gypi on cold macOS/Windows runners. * ci: skip node-gyp header pre-seed on Linux * ci: invoke node-gyp via its JS entrypoint for Windows compat (cherry picked from commit f7ba340)
Summary
npm_config_devdir, so parallel yarn postinstall builds of Electron's spec native-addon fixtures stop racing over$HOME/.cache/node-gyp/<version>/and corruptingnode_api_types.h.docker buildusing a lockfile-pinnednode-gyp, thenCOPY'd into every image at/opt/node-gyp-headers. Headers are arch-independent on Linux (the arch-specificnode.libis Windows-only), so generating once on the amd64 runner is fine for the arm images too..yarnrc.yml+ vendored release +packageManager) to pinnode-gyp.Test plan